Claude finished @carlos-alm's task —— View job

PR Review — fix(perf): pass symbolsOnly through parseFilesWasmInline

• Gather context and read changed files
• Phase 1: Root cause analysis
• Phase 2: Configuration change detection
• Phase 3: Test integrity
• Phase 4: Code quality
• Phase 5: Backlog compliance
• Final assessment

Root Cause Analysis

Problem Being Solved: parseFilesWasmForBackfill accepted a symbolsOnly flag to skip CFG/complexity/dataflow/AST visitors, but this only applied to the worker-pool path (>16 files). The inline path (parseFilesWasmInline, triggered for ≤16 files) didn't accept the parameter at all — it unconditionally set _tree on extracted symbols, which causes runAnalyses to fire all AST visitor walks. Since single-file incremental rebuilds always use the inline path, the perf optimization from PR #1436 was effectively a no-op for the most common rebuild case.

Why This Approach: The fix is minimal and correct — add symbolsOnly = false as a parameter to parseFilesWasmInline, then conditionally skip the _tree assignment. The parseFilesWasmForBackfill router already had the right interface; it just wasn't forwarding the value into the inline path. One-line caller change + ~12 lines in the callee.

Risk Assessment: Low. The change path is parseFilesWasmForBackfill → parseFilesWasmInline → conditional _tree skip. The symbolsOnly callers (native-orchestrator.ts) are already passing { symbolsOnly: true } for the this/super dispatch post-pass. The only new behavior is that small-batch inline calls now skip _tree when the caller wants them to. The dropped-language backfill path correctly leaves symbolsOnly unset (calling without opts), so that path retains full analysis data as required.

Phase 2: Configuration Change Detection

.claude/settings.json (+10/-0): Adds two new PostToolUse hooks (track-bash-writes.sh, track-moves.sh, post-git-ops.sh) and wires snapshot-pre-bash.sh as a PreToolUse Bash hook. These are developer tooling enhancements to the session tracking/audit infrastructure, not linting rules relaxed or thresholds lowered. ✓

.claude/hooks/snapshot-pre-bash.sh (ADDED), .claude/hooks/track-bash-writes.sh (ADDED): New hooks that snapshot git status --porcelain before/after Bash tool calls to detect files written by Bash commands. Both exit 0 (informational only). ✓

.github/workflows/perf-canary.yml (ADDED): A new, lightweight per-PR CI workflow that runs incremental benchmarks with BENCH_CANARY=1 (50% regression threshold). Scoped to paths that caused the v3.12.0 perf regression (src/extractors/**, src/domain/graph/**, crates/**). Appropriate and additive. ✓

tests/benchmarks/regression-guard.test.ts: Adds BENCH_CANARY mode, WASM_TIMING_THRESHOLD widening, and SIZE_METRICS — structural improvements to the guard's tolerance model. No test assertions are weakened; WASM timing tolerances are widened based on documented empirical variance, while native timings keep their strict thresholds. ✓

Phase 3: Test Integrity

tests/unit/visitor.test.ts (+0/-1): Removes // biome-ignore lint/... suppression. The suppressed comment was dead weight after a prior refactor — removing it is a clean-up, not a weakening. ✓

tests/benchmarks/regression-guard.test.ts: Adding BENCH_CANARY mode is gating-additive (new CI coverage) not softening. Adding WASM_TIMING_THRESHOLD = 0.75 and SIZE_METRICS is well-reasoned with referenced empirical data (run IDs provided inline). The existing KNOWN_REGRESSIONS entries are pruned appropriately; new entries for 3.12.0:Full build and 3.12.0:1-file rebuild include detailed root-cause comments with local A/B measurements. ✓

Coverage gap: There are no new unit tests specifically for the parseFilesWasmInline(symbolsOnly=true) code path. The fix is behavioral (skipping _tree assignment) and integration-tested indirectly via the incremental benchmark canary, but a dedicated unit/integration test asserting that symbolsOnly=true produces symbols without triggering visitor walks would be valuable for regression prevention.

Phase 4: Code Quality

src/domain/parser.ts — parseFilesWasmInline (lines 1207–1238):
The implementation is clean and correct. The symbolsOnly default-false parameter is typed and documented. The conditional:

if (!symbolsOnly) {
  symbols._tree = extracted.tree;
}

correctly gates the _tree assignment. The WASM tree object is still held in extracted.tree regardless and will be GC'd; no WASM memory leak introduced since _tree was only a reference and not the sole retention path.

⚠️ Minor concern: When symbolsOnly=true, the WASM tree object (extracted.tree) is parsed but immediately becomes unreachable without an explicit .delete() call. web-tree-sitter trees are backed by WASM linear memory and require explicit disposal to avoid leaks (the existing code at line 1078 in wasmExtractSymbols already handles the extractor-error case). For the symbolsOnly=true path, the tree is parsed but never assigned — it will leak until GC fires. At incremental rebuild frequency (1–5 files/rebuild) this is likely negligible, but it's inconsistent with the codebase's existing tree-disposal discipline.

crates/codegraph-core/src/ast_analysis/complexity.rs: The BranchAction::NotHandled variant removal is a clean refactor — classify_branch is now guaranteed to always return Handled, making the enum a newtype wrapper. The if let → irrefutable let change is idiomatic Rust. Behavior is unchanged. ✓

crates/codegraph-core/src/ast_analysis/cfg.rs: Removes a dead private method start_line_of. Clean-up. ✓

crates/codegraph-core/src/ast_analysis/dataflow.rs: Removes the callee field from LocalSource::CallReturn and LocalSource::Destructured variants. These appear to have been stale data (the callee was stored but the enum variants were only matched structurally LocalSource::Destructured { .. }). The removal eliminates the dead field. ✓

crates/codegraph-core/src/extractors/clojure.rs: Adds #[allow(unused_assignments)] to suppress the Rust lint for the next_ns_owned pattern used to extend a borrow lifetime within the match. The comment is accurate and the allow is scoped tightly. ✓

scripts/benchmark.ts: Adds --warmup flag handling to stabilize timing measurements. ✓

.claude/skills/titan-run/SKILL.md: Minor documentation update. ✓

.gitignore: Adds crates/codegraph-core/index.d.ts, .js, .node napi build artifacts. ✓

Phase 5: Backlog Compliance

• Zero-dep: ✓ No new runtime dependencies added.
• Foundation-aligned: ✓ Pure local optimization — no cloud calls, no new infrastructure, no agent behavior.
• Problem-fit: 4/5 — Directly reduces incremental rebuild latency for AI-assisted workflows. Codegraph is its own primary user; faster incremental rebuilds mean hooks complete faster, reducing friction in the development loop. The optimization was already declared in fix(perf): consolidate extractor walks and scope native post-passes (v3.12.0 publish gate) #1436 but silently not applied to the most common path.
• Breaking: No — Purely internal optimization; no API/CLI/DB schema change. Callers of parseFilesWasmForBackfill are unaffected; the symbolsOnly semantics are unchanged.
• Tier: 1

Critical Concerns

1. WASM tree leak on symbolsOnly=true inline path (minor, not blocking):
In parseFilesWasmInline at src/domain/parser.ts:1224-1232, when symbolsOnly=true, wasmExtractSymbols parses a full WASM tree (via parser.parse(code)) and returns { symbols, tree, langId }. The tree object is then discarded (not assigned to symbols._tree). Since web-tree-sitter trees are WASM-backed, this is a minor leak. At 1-file-incremental scale the impact is negligible, but it's worth a if (symbolsOnly && typeof extracted.tree?.delete === 'function') extracted.tree.delete() cleanup.

2. Scope broadening of this PR:
The core fix is src/domain/parser.ts:1207-1238 (~12 lines + doc). The PR also bundles: a new CI workflow (perf-canary.yml), benchmark warmup improvements, a regression guard overhaul, two new Claude hooks, .claude/settings.json changes, Rust dead-code cleanup, Clojure lint suppression, and a gitignore update. These are all legitimate and coherent, but CLAUDE.md recommends "One PR = one concern." In practice all changes are additive/cleanup and don't muddy the correctness review; the core fix is easily isolable.

Final Recommendation

## Root Cause Analysis
Problem Being Solved: symbolsOnly flag was ignored for ≤16-file batches using the inline WASM path
Why This Approach: Minimal, correct fix — forward the flag into parseFilesWasmInline
Risk Assessment: Low — additive change, no API/behavior change for existing callers

## Backlog Compliance
- Zero-dep: ✓
- Foundation-aligned: ✓
- Problem-fit: 4/5 — directly speeds up common incremental rebuilds
- Breaking: No
- Tier: 1

## Critical Concerns
1. Minor: Parsed WASM tree is leaked when symbolsOnly=true in inline path (no .delete() call)
2. Style: PR bundles several unrelated improvements; CLAUDE.md prefers one concern per PR
   (not blocking — all changes are coherent and defensible)

## Final Recommendation
Rating: ⭐⭐⭐⭐ (4/5)
Action: APPROVE WITH CONDITIONS
Reasoning: The core fix is correct, well-scoped, and addresses a real perf gap.
The WASM tree leak is minor but worth fixing before merge. Everything else is
additive quality improvement. Consider extracting the hook/settings changes into
a separate PR on the next opportunity to stay aligned with CLAUDE.md scope discipline.

Greptile Summary

This PR fixes a silent no-op: parseFilesWasmForBackfill accepted a symbolsOnly flag but failed to pass it into parseFilesWasmInline, so small batches (≤16 files — the common case for 1-file incremental rebuilds) always ran the full AST/CFG/complexity/dataflow visitor walk even when callers only needed symbol/call extraction.

• src/domain/parser.ts: Adds symbolsOnly = false to parseFilesWasmInline, gates the _tree assignment on !symbolsOnly, and explicitly calls .delete() on the WASM-backed tree when skipping it to avoid leaking WASM heap memory. parseFilesWasmForBackfill now forwards opts.symbolsOnly to the inline path.
• .github/workflows/perf-canary.yml: Adds src/domain/parser.ts to the paths: filter so the perf canary fires on future changes to this file.

Confidence Score: 5/5

Safe to merge — the change is a minimal, targeted parameter plumb with correct default semantics and proper WASM memory disposal.

The inline path now correctly skips the _tree assignment and immediately frees the WASM-backed tree when symbolsOnly=true, matching the already-proven worker-pool path behaviour. The undefined passthrough from opts.symbolsOnly is handled correctly by the symbolsOnly = false default. No existing call sites are affected.

No files require special attention.

Important Files Changed

Sequence Diagram

sequenceDiagram
    participant Caller
    participant parseFilesWasmForBackfill
    participant parseFilesWasmInline
    participant parseFilesWasm_workerPool as parseFilesWasm (worker pool)
    participant wasmExtractSymbols

    Caller->>parseFilesWasmForBackfill: "filePaths, rootDir, {symbolsOnly}"

    alt "filePaths.length <= 16 (inline path)"
        parseFilesWasmForBackfill->>parseFilesWasmInline: filePaths, rootDir, symbolsOnly
        loop each file
            parseFilesWasmInline->>wasmExtractSymbols: parsers, filePath, code
            wasmExtractSymbols-->>parseFilesWasmInline: symbols, tree, langId
            alt "symbolsOnly = false"
                parseFilesWasmInline->>parseFilesWasmInline: "symbols._tree = extracted.tree"
            else "symbolsOnly = true"
                parseFilesWasmInline->>parseFilesWasmInline: extracted.tree.delete()
            end
        end
        parseFilesWasmInline-->>Caller: Map relPath to ExtractorOutput
    else "filePaths.length > 16 (worker-pool path)"
        parseFilesWasmForBackfill->>parseFilesWasm_workerPool: filePaths, rootDir, mode
        parseFilesWasm_workerPool-->>Caller: Map relPath to ExtractorOutput
    end

_{Reviews (7): Last reviewed commit: "chore: merge origin/main into fix/symbol..." | Re-trigger Greptile}

Codegraph Impact Analysis

2 functions changed → 10 callers affected across 6 files

• parseFilesWasmInline in src/domain/parser.ts:1207 (6 transitive callers)
• parseFilesWasmForBackfill in src/domain/parser.ts:1259 (9 transitive callers)

Addressed both concerns from the review:

1. WASM tree leak on symbolsOnly=true inline path: Added .delete() call in parseFilesWasmInline when symbolsOnly=true and the tree object exposes a delete method. Matches the existing disposal pattern used by the extractor-error path at line 1078.

2. Perf canary missing src/domain/parser.ts: Added the path to the canary paths: filter in .github/workflows/perf-canary.yml so the inline-path optimization is guarded by the canary on future parser-only changes.

@greptileai

Resolved merge conflicts with origin/main — two add/add conflicts in .claude/hooks/snapshot-pre-bash.sh and .claude/hooks/track-bash-writes.sh. Both resolved by taking the origin/main version, which adds CMD_HASH to the snapshot filename to prevent concurrent Bash calls from overwriting each other's baseline. PR is now mergeable.

Resolved new merge conflict with main (commit 5bc8196). The only conflict was an add/add in .github/workflows/perf-canary.yml — both this PR and main added the file. Resolution keeps all additions from both sides: main's permissions: {}, scripts/update-incremental-report.ts, and tests/benchmarks/regression-guard.test.ts path entries, plus this PR's src/domain/parser.ts path entry. No logic changes.

@greptileai